Could you also please add some tests to cover that new parsing logic?

What is the reason to include a query string in a base url ?

@stof the use case in a customer project is a multi lingual Drupal site where we need to make sure for each scenario that the test runs in a specific language. The only way to ensure that is with a query parameter, ?language=es as an example.

@aik099 can you direct me too some samples on how you'd like to have those tests being written?

What is the reason to include a query string in a base url ?

@stof , I guess for parameters from that query string to be present on every built url.

@aik099 can you direct me too some samples on how you'd like to have those tests being written?

@jurgenhaas , for this particular project I have no idea, because I contributed mostly to Mink itself and it's drivers. I might suggest looking at existing tests first and then creating something similar.

I've tried an attempt for a test but without knowing which website is going to be tested I wonder which paths should be tested and how to make that test generic. Also, the test depends on what the base_url is which is defined in behat.yml and I have no influence on it.

However, here is a pretty simple test and I hope to get some feedback on where to take it from here:

@parser
Feature: URL Parser
  In order to use different URL and path notations
  As a website user
  I need to make sure that the Url is parsed correctly

  Scenario: Check a URL
    Given I am not logged in
    When I am on "/login"
    Then the url should match ".*/login.*"

@jurgenhaas Actually, I think another expected result for your example could be

http://example.com/user

I think the proposed approach can lead to incomprehension, as it doesn't respect the RFCs regarding relative and absolute uris.

Rather than putting everything in a single configuration option, what would you think about injecting two new configuration options: base_fragment and base_query? These would be optional, and thus empty by default, and could be overridden.

@csarrazi You are touching on a subject that is probably true but is different to the problem we're dealing with here. The fact that the base_url in my example already contains a path portion is not really relevant to the problem I am trying to solve and - more importantly - I'm not proposing any change on how the MinkExtension should handle base_url and path fragments. I'm only concerned about queries and fragments here.

However, if what you're saying is a general issue in MinkExtension then it would of course make sense to build a central place which is dealing with all kinds of URL building with all possible scenarios.

What I mean is that if you are on a browser with base uri http://example.com/prefix/?arg=something#abc=123, and click on a link targeting /hello, you will actually be redirected to http://example.com/hello, and lose everything else.

If you target a link to hello (notice the absence of the leading / character), you will instead go to http://example.com/prefix/hello, while still loosing your query and fragment.

For your use case, I think forcing a query or in all uris which are generated by the locatePath() method, by injecting two configuration would be the best.

Let's keep in mind that Mink is a browser emulator. It should try to behave as closely as possible to the behavior of a real browser.

This would even make your PR simpler, while still solving the issue.

In the configuration, you would have something like:

base_url: http://example.com/prefix/
base_query: { language: de, abc: 1 }
base_fragment: { anchor: ~ }

In any case, that's just my two coppers.

Right now we're not talking about controlling browser. The locatePath method that is modified in this PR is used to normalize url for comparison in

So goal here is not to loose anything. The base_url involvement here isn't what causing the issues.

I'm not sure whether I'm really clear about this.

This PR induces changes for both locatePath and visitPath.

What I mean here is that, as it is now, this PR may have side-effects for people who wish to use one-time parameters (for setting the locale in the session, etc.). Parameters will thus be transferred every time a scenario will contain steps with the I visit :path expression.

Query parameters and uri fragments can be used for two things:

• Passing one-time parameters when going on the homepage (to initialize the user's context, like language, etc.), and discard them for subsequent requests.
  • i.e. http://example.com/prefix/?language=de&abc=1#anchor -> http://example.com/prefix/hello
• Passing parameters for all queries.
  • i.e. http://example.com/prefix/?laguage=de&abc=1#anchor -> http://example.com/prefix/hello?language=de&abc=1#anchor

From what I understand, this PR fixes the problem using the second approach. This means that if I wish to use the following statement

Given I am on the homepage
When I visit "/user"

Then, the ?language=de&abc=1#anchor suffix will be added to the /user uri, even though I may not want it.

Parameters will thus be transferred every time a scenario will contain steps with the I visit :path expression.

Isn't that what's the base_url is for? The base set of url parameters (query string, path, etc.) to be used with every built url without need to specify them in each scenario?

Taken from the documentation:

base_url - if you're using relative paths in your *.feature files (and you should), then this option will define which url to use as a basename for them

The documentation only describes base_url as the basename for each feature. I don't think it is very clear that base_url should be used for passing parameters for all requests. I've got nothing against this either. But I think the new behavior is not clear, in this peculiar case.

Thus the suggestion before, by adding two new options for achieving this goal.

@stof, what do you think about @csarrazi suggestion? Now after I've seen quote from documentation I understand, that adding 2 more options would be better. This will also won't introduce BC break.

Regardless, the locatePath method should do two things:

• First, remove query parameters and hashes from base_url in order to sanitize the url before merging (improved version of what it tries to achieve right now)
• Then merge url, base_query and base_fragment with the url provided as locatePath's argument.